Skip to content

bugfix: allowNonConverged warning instead of aborting#4075

Open
MelReyCG wants to merge 5 commits into
developfrom
bugfix/rey/repair-allowNonConverged
Open

bugfix: allowNonConverged warning instead of aborting#4075
MelReyCG wants to merge 5 commits into
developfrom
bugfix/rey/repair-allowNonConverged

Conversation

@MelReyCG
Copy link
Copy Markdown
Contributor

@MelReyCG MelReyCG commented Jun 4, 2026

When max sub-steps are exceeded and allowNonConverged is enabled, the solver now warns (rank 0) and continues instead of aborting (this PR assumes allowNonConverged was intended to let the simulation continue even when not converging).

@MelReyCG MelReyCG self-assigned this Jun 4, 2026
@MelReyCG MelReyCG added ci: run CUDA builds Allows to triggers (costly) CUDA jobs flag: ready for review ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Jun 4, 2026
@MelReyCG MelReyCG changed the title bugfix: allowNonConverged bugfix: allowNonConverged warning instead of aborting Jun 4, 2026
Comment thread src/coreComponents/physicsSolvers/PhysicsSolverBase.cpp Outdated
Co-authored-by: Dickson Kachuma <81433670+dkachuma@users.noreply.github.com>
@rrsettgast
Copy link
Copy Markdown
Contributor

rrsettgast commented Jun 6, 2026

This flag seems like a bad idea. Can someone explain what measure will determine that results are valid after violating the desired tolerance for the governing equations? How critical is the violation? It seems that there can be repeated violations? Can every timestep in a simulation violate and complete? What if the residual is very high? Is there a criteria for termination then?

@joshua-white
Copy link
Copy Markdown
Contributor

joshua-white commented Jun 6, 2026

What was the previous behavior if this option was enabled? How does this change interact with other checks within the physics solvers themselves, e.g.

if( !isConfigurationLoopConverged )
{
GEOS_LOG_RANK_0( "Convergence not achieved." );
if( allowNonConverged )
{
GEOS_LOG_RANK_0( "The accepted solution may be inaccurate." );
}
else
{
GEOS_ERROR( "Nonconverged solutions not allowed. Terminating...", getDataContext() );
}
}

@joshua-white
Copy link
Copy Markdown
Contributor

If we keep this, I would suggest two changes:

  1. Modify the XML key so the user is very clear they are entering a danger zone. I would suggest making the string something like DANGER_ZONE_allowNonConverged so the user has to markup their input deck with an obvious "I know what I am doing" flag.
  2. We could force modify the output file string to automatically append something like myOutputName_UNRELIABLE so that GEOS acknowledges it itself doesn't trust the results.

I can see selected situations where for debugging purposes a user may want to keep the simulation running to see what happens and gather more info, but it has to be very clear that they are taking ownership of any poor quality results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants